Rust: Non-symmetric type propagation for lub coercions #20592
Conversation
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
2 times, most recently
from
1f8f5d3 to
bfea742
Compare
hvitved
changed the title
if and matchif and match
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
3 times, most recently
from
2f501e6 to
d0b7388
Compare
hvitved
changed the title
if and match
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
2 times, most recently
from
61d6041 to
4914a3f
Compare
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
2 times, most recently
from
4a16f67 to
a1fd231
Compare
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
from
a1fd231 to
d22f70e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| toBeTested(x) and not x.isUnknown() and getBody = x.getBody() | ||
| } | ||
|
|
||
| query predicate getClosureBody(ClosureExpr x, Expr getClosureBody) { |
Check warning
Code scanning / CodeQL
Predicates starting with "get" or "as" should return a value Warning generated test
| toBeTested(x) and not x.isUnknown() and getParam = x.getParam(index) | ||
| } | ||
|
|
||
| query predicate getBody(Function x, Expr getBody) { |
Check warning
Code scanning / CodeQL
Predicates starting with "get" or "as" should return a value Warning generated test
|
|
||
| query predicate getBody(Function x, BlockExpr getBody) { | ||
| toBeTested(x) and not x.isUnknown() and getBody = x.getBody() | ||
| query predicate getFunctionBody(Function x, BlockExpr getFunctionBody) { |
Check warning
Code scanning / CodeQL
Predicates starting with "get" or "as" should return a value Warning generated test
hvitved
force-pushed
the
rust/type-inference-branch-propagation
branch
from
d22f70e to
65b706a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a type inference performance issue in Rust by preventing symmetric type propagation for constructs that use least-upper bound (LUB) coercions, such as if expressions and match arms with multiple branches.
Key Changes:
- Introduced non-symmetric type propagation for LUB coercion contexts to prevent type inference explosions
- Restricted symmetric type equality to single-branch/single-element cases
- Added a test case demonstrating the type inference explosion scenario
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Implements non-symmetric type propagation via new predicates lubCoercion and typeEqualityNonSymmetric, and restricts symmetric propagation for multi-branch constructs |
| rust/ql/test/library-tests/type-inference/main.rs | Adds test case with if expression demonstrating type inference explosion scenario |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updates expected test output reflecting reduced type inference results due to non-symmetric propagation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| lubCoercion(mid, n2, suffix) and | ||
| not lubCoercion(mid, n1, _) and |
There was a problem hiding this comment.
The predicate checks lubCoercion(mid, n2, suffix) but doesn't verify that n1 is also a child of mid that would be subject to LUB coercion. This could lead to unintended type propagation paths. Consider adding a check to ensure n1 is also a relevant child of mid when n2 is subject to LUB coercion.
There was a problem hiding this comment.
I had the same concern, it looks like n1 is bound by type but not by location in the program.
What would the Rust code this logic targets look like?
There was a problem hiding this comment.
For example, if we have
let x = if b { A } else { C }then
- Type information flows from
AandBto the entireif. - Type information does not flow from
ifto neitherAnorB. - Type information flows in both directions between
ifandx. - If type information can flow from something to
x, then it also flows toA, but only when that something is notB(and vice versa).
Does that make sense?
There was a problem hiding this comment.
Ah, I was reading typeEquality as "these types are equal" (potentially by coincidence, e.g. two unrelated u32s in the program) whereas it is in fact "these types must be equal" (by type inference). So n1 is in fact quite tightly bound by typeEquality. And I should've read the qldoc for typeEquality.
| lubCoercion(mid, n2, suffix) and | ||
| not lubCoercion(mid, n1, _) and |
There was a problem hiding this comment.
I had the same concern, it looks like n1 is bound by type but not by location in the program.
What would the Rust code this logic targets look like?
2 participants
Rust applies least-upper bound coercions for certain constructs, such as the branches of an
ifexpression, which means that is not sound to propagate type information directly between branches. In fact, doing so can lead to type inference explosions, as witnessed by the example added on this PR.Instead of attempting to model lub coercions, we simply prevent type information to propagate from one branch to another by going directly via the type of the
ifexpression (and similar for other constructs with lub coercions).DCA looks great; we recover the performance lost on #20282 for
ruff,rendiation, andcoreutils.